Skip to content

Mark class_exists, interface_exists, trait_exists, and enum_exists as having no side effects in function metadata#5607

Merged
staabm merged 9 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-625e02d
May 9, 2026
Merged

Mark class_exists, interface_exists, trait_exists, and enum_exists as having no side effects in function metadata#5607
staabm merged 9 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-625e02d

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

@phpstan-bot phpstan-bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is semantic wrong because those function depends on other parameters.
One *_exists function could return false and then true later.

But there is something specific to them, as soon as they return true, it stay true. Once a class is loaded in PHP, it cannot be unloaded within the same process.

@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. Here's a summary of what changed:

Problem: The reviewer correctly pointed out that class_exists, interface_exists, trait_exists, and enum_exists DO have side effects (they trigger autoloading), so marking them as hasSideEffects => false in functionMetadata.php was semantically wrong.

Root cause: When rememberPossiblyImpureFunctionValues is false, TypeSpecifier::createForExpr() blocks type narrowing for any function without hasSideEffects = no. The ClassExistsFunctionTypeSpecifyingExtension calls typeSpecifier->create() with a FuncCall, which hits this impurity check and returns empty SpecifiedTypes.

Fix: Instead of lying about side effects, wrap the FuncCall in AlwaysRememberedExpr in the extension. This is an existing mechanism that bypasses the impurity check in createForExpr() (since AlwaysRememberedExpr is not a FuncCall), and when applied to the scope via MutatingScope::specifyExpressionType(), it propagates the type to the inner FuncCall too. This way isInClassExists() still finds the stored type when checking class existence.

Changes:

  • Reverted all 4 hasSideEffects => false entries from functionMetadata.php
  • Modified ClassExistsFunctionTypeSpecifyingExtension to wrap the FuncCall in AlwaysRememberedExpr
  • Reverted the unnecessary test change in AutoloadSourceLocatorTest.php
  • Kept the regression test Bug8579Test.php (still passes)

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add test to ensure that a false result for class_exists is still not always remembered when we don't remember possibly impure function.

@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Pushed. Here's what I added to address the review:

  • New test method testClassExistsFalseNotAlwaysRemembered() in Bug8579Test.php
  • New test data file bug-8579-false-not-remembered.php with two scenarios:
    1. class_exists returns false (falsy if branch not exiting), then a second class_exists for the same class still works and allows instantiation
    2. Same pattern but using if/else — the else branch sees class_exists as false, but after the block, a subsequent class_exists check can still return true

Both scenarios run with rememberPossiblyImpureFunctionValues: false and verify no errors are reported, ensuring the false result from class_exists() is not permanently cached. All tests (11996) and make phpstan pass.

Comment thread tests/PHPStan/Analyser/Bug8579Test.php Outdated

use PHPStan\Testing\PHPStanTestCase;

class Bug8579Test extends PHPStanTestCase
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need such new test? can't we test it with a regular rule-test?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Here's what I changed to address staabm's review:

  1. Converted Bug8579Test to a standard rule test: Created InstantiationDoNotRememberPossiblyImpureValuesRuleTest extending RuleTestCase<InstantiationRule> with rememberPossiblyImpureFunctionValues: false config, following the same pattern as MatchExpressionDoNotRememberPossiblyImpureValuesRuleTest.

  2. Converted the false-not-remembered test to assertType: Added two assertType('bool', class_exists(...)) assertions to the existing do-not-remember-possibly-impure-function-values.php test data file, verifying that a class_exists false result is not permanently remembered after branch merges.

  3. Deleted the old custom test: Removed Bug8579Test.php and its data files (bug-8579.php, bug-8579-false-not-remembered.php) from the Analyser test directory.

All 11997 tests pass and make phpstan reports no errors.

Comment on lines +9 to +10
// The false result must not be permanently remembered,
// so a subsequent class_exists check can still return true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be tested via assertType() in a nsrt test?

// class_exists returned false here, but we don't exit
}

assertType('bool', class_exists('Bug8579FalseNotRememberedA'));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also add assert inside the if

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Added assertType('bool', class_exists(...)) assertions inside the if branches as requested by VincentLanglet. The expected type is bool (not true/false) because with rememberPossiblyImpureFunctionValues: false, function call results are not narrowed within branches — only the class existence narrowing (used by instantiation checks) is preserved via AlwaysRememberedExpr. All 12000 tests pass and make phpstan reports no errors.

@VincentLanglet VincentLanglet requested a review from staabm May 7, 2026 07:22
@VincentLanglet VincentLanglet self-assigned this May 7, 2026
Comment thread src/Type/Php/ClassExistsFunctionTypeSpecifyingExtension.php
}
}

function testClassExistsFalseNotRemembered(): void
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these nsrt tests did not fail before this PR. is it intentional?

should we have this very same test-lines in a nsrt test with rememberPossiblyImpureFunctionValues: false?

Comment thread src/Type/Php/ClassExistsFunctionTypeSpecifyingExtension.php
Comment thread src/Node/Expr/AlwaysRememberedExpr.php Outdated
Comment on lines +11 to +12
* Wraps an expression so its type is remembered in the scope even when
* `rememberPossiblyImpureFunctionValues` is false.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that's the only use case.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced the verbose, class_exists-specific PHPDoc on AlwaysRememberedExpr with a single-line generic description: "Wraps an expression so its type is always remembered in the scope, bypassing impurity checks." This addresses VincentLanglet's review that the doc should describe the general purpose, not focus on a specific use case. All 12006 tests pass and make phpstan reports no errors.

Comment thread src/Node/Expr/AlwaysRememberedExpr.php Outdated
Comment on lines +14 to +22
* TypeSpecifier::createForExpr() returns empty SpecifiedTypes for impure
* function calls when that setting is off. Wrapping the call in this node
* bypasses that check (since AlwaysRememberedExpr is not a FuncCall) while
* MutatingScope::specifyExpressionType() propagates the type to the inner
* expression as well.
*
* Used for function calls whose result should always participate in type
* narrowing regardless of purity — e.g. class_exists() guards that gate
* "class not found" errors.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The phpdoc of this class should be generic about his purpose and not focus on a really specific usage.

@staabm staabm force-pushed the create-pull-request/patch-625e02d branch from 36db3d4 to 5ca07a2 Compare May 8, 2026 08:02
VincentLanglet and others added 9 commits May 9, 2026 10:27
…ists` as having no side effects in function metadata

- Add `class_exists`, `interface_exists`, `trait_exists`, and `enum_exists` to
  `resources/functionMetadata.php` with `hasSideEffects => false`, matching the
  existing `function_exists` entry
- When `rememberPossiblyImpureFunctionValues` was false, the type narrowing from
  these functions was blocked in `TypeSpecifier::createForExpr()` because they
  lacked metadata and defaulted to `hasSideEffects = maybe`
- Update `AutoloadSourceLocatorTest` to use the return value of `class_exists()`
  to avoid the now-triggered `function.resultUnused` rule
…pe narrowing

Instead of incorrectly marking class_exists, interface_exists, trait_exists,
and enum_exists as having no side effects (they trigger autoloading),
wrap the FuncCall in AlwaysRememberedExpr in ClassExistsFunctionTypeSpecifyingExtension.
This bypasses the rememberPossiblyImpureFunctionValues check in TypeSpecifier::createForExpr()
while keeping the correct hasSideEffects semantics.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When rememberPossiblyImpureFunctionValues is false, a false result from
class_exists() must not be permanently cached — a subsequent class_exists()
call for the same class should still be able to return true (since the
class could be autoloaded between calls).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move the Bug8579 regression test from a custom Analyser test to a
standard RuleTestCase for InstantiationRule with
rememberPossiblyImpureFunctionValues: false config.

Convert the class_exists false-not-remembered test to assertType
assertions in the existing DoNotRememberPossiblyImpureFunctionValues
test data file.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rtions

- Add interface_exists, trait_exists, and enum_exists guard tests to
  bug-8579.php rule test data, matching the existing class_exists test
- Fix nsrt assertions in do-not-remember test to use fully qualified
  \class_exists() so the expression key matches the stored type from
  AlwaysRememberedExpr — the truthy-branch assertType('true') now
  correctly fails without the fix and passes with it
- Replace tests that passed trivially (asserting 'bool' with
  unqualified class_exists, which never matched the stored key)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…memberPossiblyImpureFunctionValues: false

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@staabm staabm force-pushed the create-pull-request/patch-625e02d branch from 5ca07a2 to e69564b Compare May 9, 2026 08:27
@staabm
Copy link
Copy Markdown
Contributor

staabm commented May 9, 2026

Thank you!

@staabm staabm merged commit ecf16da into phpstan:2.1.x May 9, 2026
658 of 659 checks passed
@staabm staabm deleted the create-pull-request/patch-625e02d branch May 9, 2026 08:42
staabm pushed a commit that referenced this pull request May 9, 2026
…ists` as having no side effects in function metadata (#5607)

Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants